fix: when drag just one app in folder to launchpad quickly will core…#710
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Feb 27, 2026
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures that when a folder is removed, its associated ItemsPage disconnects and is safely destroyed after all model references are cleaned up, preventing crashes triggered by lingering signal connections from a deleted folder. Sequence diagram for updated removeFolder processsequenceDiagram
participant Caller
participant ItemArrangementProxyModel
participant ItemsPage
participant TopLevelModel
participant FolderModel
Caller->>ItemArrangementProxyModel: removeFolder(idNumber)
ItemArrangementProxyModel->>ItemArrangementProxyModel: build fullId
ItemArrangementProxyModel->>ItemArrangementProxyModel: assert m_folders.contains(fullId)
ItemArrangementProxyModel->>ItemsPage: page = m_folders.value(fullId)
ItemArrangementProxyModel->>ItemsPage: disconnect(this)
ItemArrangementProxyModel->>TopLevelModel: removeItem(fullId)
ItemArrangementProxyModel->>FolderModel: findItems(fullId)
FolderModel-->>ItemArrangementProxyModel: result
ItemArrangementProxyModel->>FolderModel: removeRows(row, 1)
ItemArrangementProxyModel->>ItemArrangementProxyModel: m_folders.remove(fullId)
ItemArrangementProxyModel->>ItemsPage: deleteLater()
ItemsPage-->>ItemArrangementProxyModel: scheduled for deferred deletion
Class diagram for ItemArrangementProxyModel folder and ItemsPage cleanupclassDiagram
class ItemArrangementProxyModel {
- QHash~QString, ItemsPage*~ m_folders
- QStandardItemModel m_folderModel
- TopLevelModel* m_topLevel
+ void removeFolder(QString idNumber)
}
class ItemsPage {
<<QObject>>
+ void disconnect(QObject* sender)
+ void deleteLater()
}
class TopLevelModel {
+ void removeItem(QString fullId)
}
class QStandardItemModel {
+ QList~QStandardItem*~ findItems(QString text)
+ void removeRows(int row, int count)
}
ItemArrangementProxyModel "1" o--> "*" ItemsPage : owns_via_m_folders
ItemArrangementProxyModel --> TopLevelModel : uses
ItemArrangementProxyModel --> QStandardItemModel : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider using
auto *page = m_folders.take(fullId);instead ofvalue()plus a separateremove()to both avoid a second hash lookup and make it explicit that the pointer is removed from the map at the same time it is retrieved. - Given the crash origin, it may be safer to assert that
pageis non-null (e.g.Q_ASSERT(page);) after retrieving it fromm_foldersto catch any unexpected cases where the map contains a null entry.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using `auto *page = m_folders.take(fullId);` instead of `value()` plus a separate `remove()` to both avoid a second hash lookup and make it explicit that the pointer is removed from the map at the same time it is retrieved.
- Given the crash origin, it may be safer to assert that `page` is non-null (e.g. `Q_ASSERT(page);`) after retrieving it from `m_folders` to catch any unexpected cases where the map contains a null entry.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
wjyrich
force-pushed
the
fix-dumpWithFolder
branch
2 times, most recently
from
64dc018 to
2eea0d3
Compare
BLumia
previously approved these changes
Feb 27, 2026
…ump. 修复文件夹只有一个应用时，快速拖拽到启动器会崩溃问题。 原因为： 文件夹在被删除后，他的ItemsPage 对象仍然存活（QObject 子对象，未被删除），连接也未断开。如果后续有任何东西触发了这个 "幽灵" page 的 pageCountChanged，仍然会发出 folderPageCountChanged 信号指向一个已不存在的文件夹。 修复方案： 断开Itempage的连接后，再进行删除 folder。
wjyrich
force-pushed
the
fix-dumpWithFolder
branch
from
2eea0d3 to
eeada85
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review这段代码的 1. 语法逻辑分析
2. 代码质量分析
3. 代码性能分析
4. 代码安全分析
改进后的代码建议基于以上分析，建议修改如下： void ItemArrangementProxyModel::removeFolder(const QString &idNumber)
{
QString fullId("internal/folders/" + idNumber);
// 检查 ID 是否存在，防止 Release 模式下崩溃
if (!m_folders.contains(fullId)) {
return;
}
// take 会自动移除键值对，并返回指针
auto *page = m_folders.take(fullId);
// 断开信号槽连接，并检查空指针
if (page) {
page->disconnect(this);
// 假设 page 不是 QObject 树的一部分，需要手动删除以防止内存泄漏
// 如果 page 的父对象设置了且会自动删除，则不需要下面这行
delete page;
}
m_topLevel->removeItem(fullId);
QList<QStandardItem*> result = m_folderModel.findItems(fullId);
// 检查查找结果是否有效
if (!result.isEmpty()) {
m_folderModel.removeRows(result.first()->row(), 1);
}
// 删除了冗余的 m_folders.remove(fullId);
}总结这段代码的主要意图是修复潜在的信号槽连接问题，但引入了内存泄漏风险和冗余的删除操作。建议按照上述改进代码进行修复，以确保逻辑正确、资源释放安全且无冗余操作。 |
BLumia
approved these changes
Feb 27, 2026
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: behind) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…dump.
修复文件夹只有一个应用时，快速拖拽到启动器会崩溃问题。
原因为： 文件夹在被删除后，他的ItemsPage 对象仍然存活（QObject 子对象，未被删除），连接也未断开。如果后续有任何东西触发了这个 "幽灵" page 的 pageCountChanged，仍然会发出 folderPageCountChanged 信号指向一个已不存在的文件夹。
修复方案： 断开Itempage的连接后，再进行删除 folder。
Summary by Sourcery
Bug Fixes: